feat(ourlogs): fetch not-yet-available pinned logs and invalidate on filter changes#116614
Merged
JoshuaKGoldberg merged 3 commits intoJun 5, 2026
Merged
Conversation
Contributor
📊 Type Coverage Diff✅ no issues found |
ebdd212 to
24d8526
Compare
24d8526 to
00fd77e
Compare
…filter changes Adds data fetching for pinned log rows that are in the URL but not yet loaded in the infinite scroll table (e.g. the virtualizer hasn't reached them yet). Also removes pinned IDs from the URL when they can't be found within the current page filters (date range, project, environment). - Add removePinnedRow to LogsPinning interface (idempotent removal) - New usePinnedLogsQuery hook: fetches missing pinned log rows via the events API, then removes any IDs that aren't returned (invalidation) - PinnedLogs now accepts fetchedPinnedRows/isFetchingPinnedRows props, merges fetched rows with existing allRows, and shows a loading indicator while rows are being fetched - logsInfiniteTable calls usePinnedLogsQuery and passes results down Refs LOGS-781 Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
00fd77e to
394b9bc
Compare
Replace removePinnedRow with removePinnedRows so invalidation removes all not-yet-available pinned ids in a single state update. Calling the single-id setter in a loop dropped removals: nuqs runs setState updaters at render, so each synchronous call saw the same committed state and only the last write survived. Include the utc flag in the pinned-log fetch for absolute date ranges so it matches the main logs query's time window and avoids spurious pin invalidation. Also fix pre-existing typecheck failures (LogFixture missing ORGANIZATION_ID, PinnedLogs wrapper row type) and a require-await lint error in the specs. Refs LOGS-781 Co-Authored-By: Claude <noreply@anthropic.com>
…-not-yet-available-pinned-logs
Member
Author
|
@cursor review |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 90715c1. Configure here.
narsaynorath
approved these changes
Jun 4, 2026
Member
Author
Great spot! No, I hadn't noticed 🙈. Filed: LOGS-837 |
Member
Author
|
Oh, dangit, my rename commit push failed and then I merged. Blagh. Sent: #116966. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Part 2 of 2 for log pinning implementation (part 1: #115102). Adds a new
usePinnedLogsQueryhook that sets up a query, which is then used in the existingPinnedLogscomponent.Closes LOGS-781